Minor patches to TBB Windows build for compatibility with RTools make #2999
Conversation
|
I've also left the existing usage of |
|
@WardBrian Any chance we could sneak this through in the 2.34 release? It would help simplify the windows setup steps (only need to set PATH now) @wds15 would you happen to remember the POSIX issues that led to the |
@WardBrian good call! I've updated the makefiles to first do a check for the presence of |
|
Changes look good now, thanks! As for this in the 2.34 release, I'm torn. Calling the fact that @rok-cesnovar thoughts? |
|
First of all, this is awesome. Not requiring mingw32-make is a huge huge win! But cmdstan scripts still require it right now and I think we would need to clean up the Cmdstan makefiles, cleanup docs and installation instructions before the release. We could do that, but it feels like it would be a bit rushed to me. And yeah, more testing time would be helpful as well. Asking Windows users to test the RC because of a somewhat big change would be helpful as well. The only other option is making an RC2 and prolonging the freeze period. |
Ah yeah good point. This change isn't particularly urgent, better to play it safe |
|
Agreed @rok-cesnovar, this is a great change! I had always suspected that the issues with Getting the build system out of our way is also the current barrier to using the Microsoft compilers (or at least Clang-for-MSVC) for Stan, which is a goal of mine. One day |
|
@andrjohns I only recall that |
|
@andrjohns would you mind looking into what needs to be changed in the stan and cmdstan repos? |
Summary
This PR applies some minor changes to the TBB's build rules on Windows so that it can be compiled with the version of
makebundled with RTools - users will no longer need to additionally installmingw32-makeon windows.These changes are based on
RcppParallel's changes.I've run a GHA workflow which shows that the Math library & TBB successfully build with these changes under RTools40, RTools42, and RTools43 - without needing to install any additional packages
Tests
N/A - existing tests should still pass, testing workflow already run to confirm successful build
Side Effects
N/A
Release notes
Updated TBB Windows build rules for compatibility with RTools make
Checklist
Copyright holder: Andrew Johnson
The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
- Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
- Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)
the basic tests are passing
./runTests.py test/unit)make test-headers)make test-math-dependencies)make doxygen)make cpplint)the code is written in idiomatic C++ and changes are documented in the doxygen
the new changes are tested